refactor: improve GatorPermissionsController permission storage and syncronisation#7847
Conversation
7684e9e to
f1dd835
Compare
GatorPermissionsController permission syncronisation and storage
93b4ebd to
212df20
Compare
5a1282e to
bbeffdd
Compare
packages/gator-permissions-controller/src/GatorPermissionsController.test.ts
Outdated
Show resolved
Hide resolved
bbeffdd to
9fbbbd0
Compare
GatorPermissionsController permission syncronisation and storageGatorPermissionsController permission syncronisation and storage
GatorPermissionsController permission syncronisation and storageGatorPermissionsController permission storage and syncronisation
…ermission storage - Replace `gatorPermissionsMapSerialized` state with `grantedPermissions` array (`PermissionInfoWithMetadata[]`) - Change `GatorPermissionsController` constructor to accept `config` object - Replaced `isGatorPermissionsEnabled` with `enabledPermissionTypes` property and removed related `GatorPermissionsNotEnabledError` - Updated `fetchAndUpdateGatorPermissions` to be parameterless - Replace `getPendingRevocations` with `isPendingRevocation(permissionContext);` as an interim change to moving revocation entirely internal to the controller - Add `executeSnapRpc` utility function to handle Snap RPC requests - Drop custom permission support from types, mocks, and mock tests - Add test coverage for specific controller error types
- adds 'initialize()' function that will fetch data if none is available, or if no syncronisation has occurred recently - multiple calls to 'fetchAndUpdateGatorPermissions()' will no longer trigger additional queries
d17bc58 to
eca7605
Compare
| */ | ||
| isGatorPermissionsEnabled: boolean; | ||
|
|
||
| supportedPermissionTypes: SupportedPermissionType[]; |
There was a problem hiding this comment.
Should supportedPermissionTypes be non-empty? It might be safer to enforce this at the type level to prevent invalid states.(e.g. [SupportedPermissionType, ...SupportedPermissionType[]])
There was a problem hiding this comment.
I think it's valid to have no supported permission types - this would effectively mean Advanced Permissions is not enabled (which is the case up until now in main dist).
| - Constructor requires `config` with `supportedPermissionTypes`; optional `gatorPermissionsProviderSnapId` and `state`; enable/disable flow removed | ||
| - Adds `initialize()` function that performs a syncronisation process if required when the controller is first initialized | ||
| - State: `grantedPermissions` (array of `PermissionInfoWithMetadata`) replaces `gatorPermissionsMapSerialized`; `isGatorPermissionsEnabled` removed | ||
| - `fetchAndUpdateGatorPermissions()` no longer accepts parameters | ||
| - `getPendingRevocations` / `pendingRevocations` getter replaced by `isPendingRevocation(permissionContext)`; list on `state.pendingRevocations` | ||
| - Messenger: removed `EnableGatorPermissions` and `DisableGatorPermissions` actions; added `isPendingRevocation` action | ||
| - Removed exports: `serializeGatorPermissionsMap`, `deserializeGatorPermissionsMap`, `GatorPermissionsNotEnabledError`, `CustomPermission`, `PermissionTypesWithCustom`, `PermissionResponseSanitized`, `StoredGatorPermissionSanitized`, `GatorPermissionsMap`, `SupportedGatorPermissionType`, `GatorPermissionsMapByPermissionType`, `GatorPermissionsListByPermissionTypeAndChainId`, `GatorPermissionsControllerErrorCode.GatorPermissionsNotEnabled` | ||
| - Added exports: `GatorPermissionsControllerConfig`, `PermissionInfo`, `PermissionInfoWithMetadata`, `SupportedPermissionType` |
There was a problem hiding this comment.
This is really verbose for a changelog entry. Maybe worth synthesizing more.
There was a problem hiding this comment.
Good call - there's a lot of change in there! I've tried to make this more concise.
| - Replaces `gatorPermissionsMapSerialized` with `grantedPermissions` property in internal state, replaces related types, and utility functions | ||
| - `fetchAndUpdateGatorPermissions()` no longer accepts parameters and resolves to `void` | ||
| - `getPendingRevocations` / `pendingRevocations` getter replaced by `isPendingRevocation(permissionContext)`; list on `state.pendingRevocations` | ||
| - Bump `@metamask/transaction-controller` from `^62.11.0` to `^62.17.0` ([#7775](https://github.com/MetaMask/core/pull/7775), [#7802](https://github.com/MetaMask/core/pull/7802), [#7832](https://github.com/MetaMask/core/pull/7832), [#7854](https://github.com/MetaMask/core/pull/7854), [#7872](https://github.com/MetaMask/core/pull/7872)), ([#7897](https://github.com/MetaMask/core/pull/7897))>>>>>>> Stashed changes |
There was a problem hiding this comment.
There was a problem hiding this comment.
I'll fix this in the release PR


Explanation
Makes the following changes to the
GatorPermissionsControllerto improve storage and syncronisation of permissions data with Profile Sync service:supportedPermissionTypesand optionalgatorPermissionsProviderSnapIdandmaxSyncIntervalMs.lastSyncedTimestamp(default -1); set when a sync completes successfully soinitialize()can decide when to sync again.initialize()call after construction; triggers a sync if no sync has run yet or cached data is older than the configured interval.fetchAndUpdateGatorPermissions()now returnsPromise<void>; callers should read from controller state. Concurrent calls share the same in-flight sync and no longer trigger duplicate operations.GatorPermissionsMapJSON-serialized state (and related types and utils); permissions are now stored in state as a simple array. Sync is done by calling the gator permissions Snap via RPC.enableGatorPermissions()/disableGatorPermissions()and related actions; the controller is used as the single source for gator permissions when instantiated.Checklist
Note
Medium Risk
Breaking API/state-shape changes and new sync caching logic can affect downstream consumers and initial data freshness if integration assumptions differ.
Overview
Refactors
GatorPermissionsControllerto be config-driven and to store permissions as a flatstate.grantedPermissionsarray (sanitized for UI) instead of a serializedGatorPermissionsMap.Adds
initialize()withlastSyncedTimestamp+ configurablemaxSyncIntervalMsto control when an initial/stale sync happens, and changesfetchAndUpdateGatorPermissions()to returnPromise<void>while deduplicating concurrent sync calls.Removes
enableGatorPermissions/disableGatorPermissions, related actions/types/utils, and drops public support for “custom” permission types; introducesexecuteSnapRpcand updates docs/tests accordingly (including new error unit tests).Written by Cursor Bugbot for commit 993b214. This will update automatically on new commits. Configure here.